Skip to content

[Dashboard navigation] Adds popout icon to links that open in new tab#165908

Closed
nickpeihl wants to merge 3 commits intoelastic:navigation-embeddablefrom
nickpeihl:navEmbeddable-external-icon
Closed

[Dashboard navigation] Adds popout icon to links that open in new tab#165908
nickpeihl wants to merge 3 commits intoelastic:navigation-embeddablefrom
nickpeihl:navEmbeddable-external-icon

Conversation

@nickpeihl
Copy link
Contributor

@nickpeihl nickpeihl commented Sep 6, 2023 •
edited
Loading

Fixes #165771

Summary

Adds an extraAction button that renders a popout icon for Dashboard or URL links that open in a new tab. This is visually similar to EuiLink with the external prop. Unfortunately, EuiListGroupItem does not support the external prop so we add the icon as an extra action button. However, EuiListGroupItem components do support truncating text and the extra action button appears regardless of the truncation.

extra-actions-with-tooltip.mp4
truncated-with-tooltips.mp4

Note, the icon will not appear on Dashboard links that open in a new tab until elastic/eui#7159 is available in Kibana. EUI has been updated!

@nickpeihl nickpeihl added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// Project:Dashboard Navigation Related to the Dashboard Navigation Project labels Sep 6, 2023
@nickpeihl nickpeihl marked this pull request as ready for review September 22, 2023 19:29
@nickpeihl nickpeihl requested a review from a team as a code owner September 22, 2023 19:29
@elasticmachine
Copy link
Contributor

elasticmachine commented Sep 22, 2023

Pinging @elastic/kibana-presentation (Team:Presentation)

@nickpeihl nickpeihl requested a review from Heenawter September 22, 2023 19:29
@kibana-ci
Copy link

kibana-ci commented Sep 22, 2023 •
edited
Loading

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #9 / Controls Range Slider Control create and edit edits title and size of an existing control and retains existing range selection
  • [job] [logs] FTR Configs #9 / Controls Range Slider Control create and edit edits title and size of an existing control and retains existing range selection
  • [job] [logs] FTR Configs #61 / dashboard app - group 1 Changing field formatter to Url applied on dashboard
  • [job] [logs] FTR Configs #61 / dashboard app - group 1 Changing field formatter to Url applied on dashboard
  • [job] [logs] FTR Configs #18 / dashboard app - group 5 embed mode non-default URL params renders as expected when scrolling
  • [job] [logs] FTR Configs #18 / dashboard app - group 5 embed mode non-default URL params renders as expected when scrolling
  • [job] [logs] Jest Tests #19 / Dashboard link component current dashboard is not a clickable href
  • [job] [logs] FTR Configs #10 / Options list control Dashboard options list creation and editing Options List Control creation and editing experience renames an existing control and retains selection
  • [job] [logs] FTR Configs #10 / Options list control Dashboard options list creation and editing Options List Control creation and editing experience renames an existing control and retains selection
  • [job] [logs] FTR Configs #21 / pluggable panel actions "after all" hook in "pluggable panel actions"
  • [job] [logs] Jest Integration Tests #1 / SO type registrations does not remove types from registrations without updating excludeOnUpgradeQuery

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
navigationEmbeddable 32.8KB 33.1KB +338.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Heenawter
Heenawter requested changes Sep 22, 2023
View reviewed changes
Copy link
Contributor

@Heenawter Heenawter left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's some rendering issues in horizontal layout:

Screenshot 2023-09-22 at 2 12 59 PM

I would expect the text to be vertically centered with the external icon - right now, it's almost rendering like a superscript :) And perhaps we could reduce the space between the icon and the text a bit? (both in horizontal + vertical layouts). What do you think @andreadelrio?

@andreadelrio
Copy link
Contributor

andreadelrio commented Sep 22, 2023

Looks like there's some rendering issues in horizontal layout:

Screenshot 2023-09-22 at 2 12 59 PM

I would expect the text to be vertically centered with the external icon - right now, it's almost rendering like a superscript :) And perhaps we could reduce the space between the icon and the text a bit? (both in horizontal + vertical layouts). What do you think @andreadelrio?

Agreed. @nickpeihl take a look at how we handle the external link icon here to get an idea of that space and alignment we should be aiming for.

@Heenawter Heenawter force-pushed the navigation-embeddable branch from d942688 to 7a3c8ef Compare September 25, 2023 15:42
@Heenawter Heenawter deleted the branch elastic:navigation-embeddable September 29, 2023 14:25
@Heenawter Heenawter closed this Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Heenawter Heenawter Heenawter requested changes

Assignees

No one assigned

Labels

Project:Dashboard Navigation Related to the Dashboard Navigation Project Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t//

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nickpeihl @elasticmachine @kibana-ci @andreadelrio @Heenawter